install: stop nested --bun from self-referencing its node shim - #30713

Overview

This PR reorders the symlink-target selection in create_fake_temporary_node_executable (src/install/lib.rs:602-647) so the /tmp/bun-node-<sha>/{node,bun} shims point at bun_core::self_exe_path() (the canonical resolved binary) rather than argv[0]. It adds a regression test in test/cli/install/bun-run.test.ts that reproduces the nested bun run --bun bun run --bun <bin> ELOOP failure from #30711.

Security risks

None introduced. The function already creates symlinks under a 0700-owned dir in shared /tmp; that hardening logic is untouched. If anything, deriving the link target from /proc/self/exe / _NSGetExecutablePath rather than caller-controlled argv[0] reduces the chance of the shim ever pointing somewhere unintended.

Level of scrutiny

Low–medium. The change is ~30 lines of control-flow reordering in a single function on the --bun slow path (marked #[cold]). I verified both call sites (run_command.rs:1969, PackageManager.rs:1200) pass an empty optional_bun_path, so the new first branch is never taken on entry and the effective change is simply "always try self_exe_path() before trusting absolute argv[0]". The out-param now feeds the canonical path into npm_execpath / load_node_js_config, which is strictly more correct. The new early-Ok(()) return when neither self_exe_path() nor an absolute argv[0] is available replaces the old behavior of symlinking to a relative argv[0], which was never useful.

Other factors

• No CODEOWNERS coverage for either file.
• Windows path is explicitly unaffected (already uses win::exe_path_w()).
• The added test is gated correctly (skipIf(isWindows)), uses existing harness patterns, and asserts both the absence of the ELOOP message and exit 0.
• The single CI failure (fetch-tcp-keepalive.test.ts on x64-asan) is a known unrelated flake per the retrigger commit.

Overview

This PR fixes #30711 by reordering the symlink-target selection in create_fake_temporary_node_executable (src/install/lib.rs). Previously the function preferred an absolute argv[0] over self_exe_path(); on a nested bun run --bun bun run --bun ..., the inner process's argv[0] is the PATH-resolved <BUN_NODE_DIR>/bun shim itself, so the symlink pointed at itself and /usr/bin/env node failed with ELOOP. The fix prefers bun_core::self_exe_path() (which readlinks /proc/self/exe on Linux and canonicalizes _NSGetExecutablePath on macOS, memoized via Once), falling back to absolute argv[0] only if that fails. A regression test in test/cli/install/bun-run.test.ts reproduces the exact reporter scenario and gate-checks the fix.

Security risks

None. The change tightens correctness of the symlink target written under /tmp/bun-node-<sha>/. The existing 0700 / owner / mode checks on that directory are unchanged. Resolving to the real executable via /proc/self/exe is strictly safer than trusting argv[0], which is caller-controlled.

Level of scrutiny

Moderate. This is on the bun run --bun / lifecycle-script slow path (the function is #[cold]), not the plain bun foo.js startup path. The logic change is ~20 lines (mostly comments) and is a conservative reordering with graceful fallback: if self_exe_path() fails (e.g., no /proc), it falls back to the old absolute-argv[0] behavior; if neither is usable it returns early without modifying PATH rather than creating a broken relative symlink (an improvement over the old code). Both callers (run_command.rs and PackageManager.rs) pass an empty optional_bun_path, so the pre-supplied-path branch is unchanged in practice. Windows is unaffected (already uses win::exe_path_w()).

Other factors

• The optional_bun_path out-param now carries the resolved exe path rather than argv[0], which feeds npm_execpath in run_command.rs — this matches what configure_env_for_run already does for npm_execpath, so it's a consistency improvement, not a regression.
• The bug-hunting system found no issues; CodeRabbit had no actionable comments.
• The regression test is well-constructed: it plants a bun symlink on PATH so the inner invocation's argv[0] is PATH-resolved, and uses a #!/usr/bin/env node shebang script in node_modules/.bin/ to surface ELOOP. The PR description confirms the test fails without the fix and passes with it.
• No CODEOWNERS entries cover these paths.

Overview

This PR fixes #30711 by reordering the symlink-target selection in create_fake_temporary_node_executable (src/install/lib.rs, POSIX-only branch). Previously the function preferred an absolute argv[0] over self_exe_path(); on nested bun run --bun bun run --bun <bin>, the inner bun's argv[0] is the shim path itself, producing a self-referencing symlink and ELOOP. The fix prefers the caller-supplied optional_bun_path, then bun_core::self_exe_path() (kernel-provided real image path, memoized via Once), and only falls back to absolute argv[0] if that fails. A regression test in test/cli/install/bun-run.test.ts reproduces the exact reporter scenario and was gate-verified (fails on pre-fix lib.rs with ELOOP/exit 126, passes with the fix). Two files changed, ~40 effective lines (mostly explanatory comments).

Security risks

None introduced. The symlink directory creation/ownership checks (mkdir 0700, lstat uid/mode guard) are unchanged. Switching the link target from caller-controlled argv[0] to the kernel-reported executable path (/proc/self/exe / _NSGetExecutablePath) is strictly safer — it removes a way for a parent process to influence what the shim points at. Windows is untouched (already uses win::exe_path_w()).

Level of scrutiny

Low–moderate. This is a small, well-scoped reorder of two existing data sources inside a single #[cfg(not(windows))] block, with no new syscalls, no new unsafe, and no change to the EEXIST/readlink/replace loop or directory hardening. The accompanying test is precise and was verified to fail without the fix. No CODEOWNERS cover these paths.

Other factors

The one inline finding is explicitly a nit: the absolute-argv[0] fallback can still self-loop, but only when self_exe_path() fails — i.e. /proc not mounted on Linux or _NSGetExecutablePath failure on macOS, both treated as effectively unreachable elsewhere in the codebase (util.rs uses .expect("unreachable") on the same call). The PR is a strict improvement on every realistic path, so this is defense-in-depth the author may optionally fold in, not a merge blocker. CI history shows only unrelated infra flakes (Windows SDE download, fetch-tcp-keepalive ASAN); the PR has been rebased twice with the gate re-verified each time.

Overview

This PR fixes #30711: nested bun run --bun bun run --bun <bin> invocations created a self-referencing symlink at $BUN_NODE_DIR/{bun,node}, causing /usr/bin/env node to fail with ELOOP. The fix reorders symlink-target selection in create_fake_temporary_node_executable (src/install/lib.rs) to prefer bun_core::self_exe_path() — the canonical, OS-resolved executable path — over argv[0], which on nested --bun is the shim path itself. A regression test in test/cli/install/bun-run.test.ts reproduces the reporter's exact invocation and has been gate-checked (fails on pre-fix lib.rs with ELOOP/exit 126, passes with the fix).

Since my previous review, the author folded in my nit: the fallback arm (taken only when self_exe_path() fails) now explicitly checks argv0.starts_with(BUN_NODE_DIR) and returns Ok(()) early rather than rewriting the outer bun's working shim into a self-loop. The inline thread is resolved and the change matches what I suggested.

Security risks

None introduced. The /tmp/bun-node-<sha> directory ownership and 0700-permission checks are untouched. If anything, preferring the OS-resolved canonical path over caller-controlled argv[0] as the symlink target is strictly safer — argv[0] is attacker-influenceable, /proc/self/exe is not.

Level of scrutiny

Moderate. This is a focused ~30-line logic reorder in one POSIX-only function, with no new syscalls or trust boundaries. The Windows path is unchanged (already uses win::exe_path_w()). The change is well-commented, the root cause is clearly documented inline with the issue number, and the test is a faithful repro of the user-reported bug.

Other factors

• No CODEOWNERS coverage for either file.
• No outstanding human-reviewer comments; coderabbit had no findings; my own prior nit is resolved.
• The author re-verified the fail/pass gate after each rebase (three times), and the test follows the existing patterns in bun-run.test.ts (tempDir, Bun.spawn, describe.concurrent, it.skipIf(isWindows)).
• One incidental behavior change: when argv[0] is an absolute non-shim path (e.g. shebang launch), the symlink now points at the canonical exe rather than argv[0] verbatim. This is the intended improvement and the linked-issues bot notes it likely fixes #4690/#7504/#27722 as well.